-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #38130 - add info to job details #939
base: master
Are you sure you want to change the base?
Conversation
# node :inputs do | ||
# # @job_invocation.pattern_template_invocations[0][:input_values] | ||
# @job_invocation.pattern_template_invocations[0].input_values | ||
# # @job_invocation.pattern_template_invocations.map(&:input_values) | ||
# # @job_invocation.pattern_template_invocations.map{ |invocation| invocation.input_values | ||
# # } | ||
# end | ||
|
||
# child @job_invocation.pattern_template_invocations => :pattern_template_invocations do |template| | ||
# puts template.inspect | ||
# # @job_invocation.pattern_template_invocations | ||
# child template.input_values => :input_values do | ||
# attributes :template_input_name, :template_input_id | ||
# node :value do |iv| | ||
# iv.template_input.respond_to?(:hidden_value) && iv.template_input.hidden_value? ? '*' * 5 : iv.value | ||
# end | ||
# end | ||
# end | ||
|
||
# child @inputs do | ||
# child :input_values do | ||
# attributes :template_input_name, :template_input_id | ||
# node :value do |iv| | ||
# iv.template_input.respond_to?(:hidden_value) && iv.template_input.hidden_value? ? '*' * 5 : iv.value | ||
# end | ||
# end | ||
# end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamruzicka I couldnt figure out how to extract the inputs from the pattern, could you advise on it?
tab_overview does
template_invocations = job_invocation.pattern_template_invocations
template_invocations.each do |template_invocation|
render :partial => 'card_user_input', :locals => { :template_invocation => template_invocation }
card_user does template_invocation.input_values.joins(:template_input).each do |input_value|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we already sort of have that in https://github.com/theforeman/foreman_remote_execution/blob/master/app/views/api/v2/job_invocations/main.json.rabl#L37 ? It takes inputs from all the template invocations, not just the pattern. In the frontend you should be able to look at the data you get and then pick the one which has null host_id.
@@ -1,4 +1,5 @@ | |||
<% template_invocations = job_invocation.pattern_template_invocations %> | |||
<%= job_invocation.pattern_template_invocations[0].input_values.inspect %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftovers to remove
// TODO is needed? bookmark_id is always null in the wizard, also why is knowing the bookmark relevent? | ||
// const targetingSelectioning = targeting.bookmark_id ? `{__('Bookmark') ${targeting.bookmark_name}` : __('Manual selection'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamruzicka _card_target_hosts does:
<% if job_invocation.bookmark.present? %>
<%= _('Bookmark') %> <%= job_invocation.bookmark.name %><br>
<% else %>
<%= _('Manual selection') %>
<% end %>
<%= _('using ') %>
<strong><%= _(Targeting::TYPES[job_invocation.targeting.targeting_type]).downcase %></strong>
<pre><%= job_invocation.targeting.search_query %></pre>
</p>
but in the job wizard we always send null in bookmarks, but users can send bookmarks in api post requests.
Should we show bookmark info here, skip it, or work later to add it?
aruzicka edit: styling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for showing it and (eventually) make the wizard send the information as well
// <ExpandableSection | ||
// toggleText={ | ||
// isExpanded ? __('Show less information') : __('Show more information') | ||
// } | ||
// onToggle={setIsExpanded} | ||
// isExpanded={isExpanded} | ||
// > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kmalyjur the design shows the expandables being inside an expandable but I think we can skip the top parent, and just show the sections?
Also named them after the cards that hold the same info, which is different from the mock
88b860a
to
9f12a9a
Compare
Adding info that was shown in
app/views/job_invocations/_card_*